Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SDK] Add 'algorithm_settings' in client tune #2227

Merged

Conversation

shipengcheng1230
Copy link
Contributor

What this PR does / why we need it:
The algorithm_settings cannot be set in the tune method of the python SDK, therefore many details here cannot be immediately used if not submitting the manifest directly.

This PR adds the capability to do it. By default, algorithm_settings is set to None which aligns with the initiation of V1beta1AlgorithmSpec. So when not setting algorithm_settings the behavior is not changed.

Which issue(s) this PR fixes:
Fixes #2225

@@ -141,6 +141,7 @@ def tune(
base_image: str = constants.BASE_IMAGE_TENSORFLOW,
namespace: Optional[str] = None,
algorithm_name: str = "random",
algorithm_settings: Union[dict, list[models.V1beta1AlgorithmSetting], None] = None,
Copy link
Member

@andreyvelich andreyvelich Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering should we discuss idea that @droctothorpe suggested here: #2225 (comment).

E.g. we can create separate Class for every Suggestion that Katib supports and add algorithm settings, search space, other features for this suggestion: katib.suggestion.optuna, katib.suggestion.skopt.
Different Suggestions might have different supported features that we can add to these classes.
For example: @DeukWoongWoo asked for callable search space support for Optuna: #2228

I understand that will add maintain overhead, but user will get better experience.
We need to discuss what is the easiest way for the user to get access for Katib Suggestion features.

What are your thoughts @johnugeorge @tenzen-y @droctothorpe @shipengcheng1230 @DeukWoongWoo @a9p @c-bata @anencore94 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are building similar configuration objects internally for stuff like dask and pytorch. Code completion, type hints, and robust docstrings make managing complex configurations a lot easier. All for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the idea for a better user experience! How do you think using pydantic for this checking/validation capability?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreyvelich
I totally agree completely with the statement: "We need to discuss what is the easiest way for the user to get access to Katib Suggestion features." However, I think it's important to note that this PR is primarily aimed at enabling the setting of algorithm settings in katib_client, which is not currently possible as mentioned in the "search-algorithms-in-detail" guide.
It's a bit unclear whether what you're trying to say is discussing features that will be added, or if you're focused on discussing how users will be able to access input into algorithm settings as this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @andreyvelich, as @DeukWoongWoo suggested, do you feel comfortable to let this PR just enable the specification and have future one to validate and give suggestions to the input of algorithm settings?

Copy link
Member

@andreyvelich andreyvelich Sep 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks everyone for your comments!

We are building similar configuration objects internally for stuff like dask and pytorch.

@droctothorpe Would you mind share some details here ? Is there something that we can adopt with Katib too ?

It's a bit unclear whether what you're trying to say is discussing features that will be added, or if you're focused on discussing how users will be able to access input into algorithm settings as this PR.

I just want to discuss how to streamline experience for users to use Suggestions that Katib supports (e.g. Optuna, Hyperopt, etc.). Currently, it's not clear which underlying framework we use for HP tuning algorithms until user goes to Search Algorithm in Detail guide.
Also, there are overlaps in supported algorithms between different frameworks: https://www.kubeflow.org/docs/components/katib/katib-config/#suggestion-settings.
E.g. tpe is supported by Hyperopt and Optuna (I assume that 2 frameworks might have different features that we can use in Katib + Kubernetes).
Thus, users need to understand relationship between Katib Experiment + Katib Config to understand the flow.

As an user experience example, in Ray tune they wrap every supported framework in a separate module (e.g. ray.tune.search.optuna) that might have custom settings.
Wondering what would be the optimal experience for Katib users.

do you feel comfortable to let this PR just enable the specification and have future one to validate and give suggestions to the input of algorithm settings?

Yeah, sure, I don't want to block this PR, just to hear your thoughts.

cc @tenzen-y @johnugeorge @gaocegege

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@droctothorpe Would you mind share some details here ? Is there something that we can adopt with Katib too ?

Happy to elaborate. Imagine something like this:

from typing import Literal, Optional

class BayesianOptimizationSettings:
    def __init__(
        base_estimator: Literal["GP", "RF", "ET", "GBRT"] = "GP",
        n_initial_points: int = 10,
        acq_func: str = "gp_hedge",
        acq_optimizer: Literal["sampling", "lbfgs", "auto"] = "auto",
        random_state: Optional[int] = None
    ):
      self.base_estimator = base_estimator
      # etc.

This would include comprehensive docstrings, code completion, and input validation, so you wouldn't have to reference an external resource outside of the SDK. It could optionally leverage ipywidgets to do something like this:

You could then instantiate the config object, edit properties on the fly (not just at instantiation time) with the benefit of code completion, type hints, and docstrings, then pass the configuration object to the tune method.

Just a thought. FWIW, it's a bigger lift (and a much bigger operational burden) than this PR and should probably not be a condition for merging it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sharing, actually, that is similar to what I proposed: #2227 (comment), but using Suggestion instead of Algorithm.
Do we want to create separate issue to track this improvement @droctothorpe @shipengcheng1230 ?

Copy link
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update @shipengcheng1230!
/lgtm
/approve

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich, shipengcheng1230

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tenzen-y
Copy link
Member

tenzen-y commented Oct 5, 2023

@kubeflow/wg-automl-leads Could you restart failed CI jobs? It seems that some jobs failed due to issues regarding reachability to the container registry.

@google-oss-prow google-oss-prow bot merged commit 50a3f41 into kubeflow:master Oct 5, 2023
59 checks passed
@shipengcheng1230 shipengcheng1230 deleted the add_algorithm_settings branch October 18, 2023 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add function of tune, a function supported by Katibclient on python sdk
5 participants